Implement support for groupshared arg attribute #8013

spall · 2025-12-18T18:50:45Z

Implements support for groupshared argument attribute.
Closes #7969

get compiler to error when types aren't exactly the same + arg not groupshared bug fix fix test disallow groupshared param in export/noinline funcs first check if there are attrs remove accidental change tests

update tests to show new mangling

github-actions · 2025-12-18T18:51:52Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 60a1c8568b405bbe9f47ab0431f627ef64988cc4 8511e4efe9344b5a326a358463f14582da38fe07 -- lib/HLSL/HLLegalizeParameter.cpp tools/clang/include/clang/AST/Decl.h tools/clang/lib/AST/HlslTypes.cpp tools/clang/lib/AST/MicrosoftMangle.cpp tools/clang/lib/CodeGen/CGHLSLMS.cpp tools/clang/lib/Sema/SemaDeclAttr.cpp tools/clang/lib/Sema/SemaHLSL.cpp tools/clang/lib/Sema/SemaOverload.cpp

View the diff from clang-format here.

diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp
index ee7724c3..200f62f6 100644
--- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp
+++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp
@@ -1247,7 +1247,7 @@ unsigned CGMSHLSLRuntime::AddTypeAnnotation(QualType Ty,
 
   if (const ReferenceType *RefType = dyn_cast<ReferenceType>(Ty))
     Ty = RefType->getPointeeType();
-  
+
   // Get size.
   llvm::Type *Type = CGM.getTypes().ConvertType(paramTy);
   unsigned size = dataLayout.getTypeAllocSize(Type);
diff --git a/tools/clang/lib/Sema/SemaHLSL.cpp b/tools/clang/lib/Sema/SemaHLSL.cpp
index 56bb346c..07fbd523 100644
--- a/tools/clang/lib/Sema/SemaHLSL.cpp
+++ b/tools/clang/lib/Sema/SemaHLSL.cpp
@@ -14485,7 +14485,7 @@ void Sema::DiagnoseHLSLDeclAttr(const Decl *D, const Attr *A) {
           switch (A->getKind()) {
           case clang::attr::HLSLGroupShared: {
             Diag(A->getLocation(), diag::err_hlsl_varmodifiersna)
-	         << "groupshared"
+                << "groupshared"
                 << "export/noinline"
                 << "parameter";
             return;

Check this box to apply formatting changes to this branch.

hekota

Mostly looks good, I just have a few comments and suggestions/nits.

hekota · 2026-01-09T01:43:58Z

tools/clang/test/SemaHLSL/v202x/groupshared/ScalarTest.hlsl

+groupshared uint16_t SharedData;
+
+void fn1(groupshared half Sh) {
+// expected-note@-1{{candidate function not viable: no known conversion from '__attribute__((address_space(3))) uint16_t' to '__attribute__((address_space(3))) half &' for 1st argument}}


Since references are not supported in HLSL, should we make sure the error includes half and not half &?

technically in this case it is a reference.

I get that internally it is a reference, but for the user half & is confusing because references are not part of the language. It would be good to use ParamType.getNonReferenceType() instead of ParamType in the error message, the same way is it used in the ASTContext::hasSameUnqualifiedType call.

hekota · 2026-01-09T01:47:39Z

tools/clang/test/SemaHLSL/v202x/groupshared/NotGroupSharedTest.hlsl

+groupshared uint16_t SharedData;
+
+void fn1(groupshared half Sh) {
+// expected-note@-1{{candidate function not viable: 1st argument ('half') is in address space 0, but parameter must be in address space}}


Suggested change

// expected-note@-1{{candidate function not viable: 1st argument ('half') is in address space 0, but parameter must be in address space}}

// expected-note@-1{{candidate function not viable: 1st argument ('half') is in address space 0, but parameter must be in address space 3}}

hekota · 2026-01-09T01:48:30Z

tools/clang/test/SemaHLSL/v202x/groupshared/NotGroupSharedTest.hlsl

+RWStructuredBuffer<uint4> Out : register(u0);
+
+groupshared uint16_t SharedData;
+


Are these needed for the test to work?

hekota · 2026-01-09T01:50:11Z

tools/clang/test/SemaHLSL/v202x/groupshared/InOutWarning.hlsl

+
+void fn2() {
+  fn1(SharedData);
+// expected-warning@-1{{assing groupshared variable to a parameter annotated with inout. See'groupshared' parameter annotation added in 202x}}


Suggested change

// expected-warning@-1{{assing groupshared variable to a parameter annotated with inout. See'groupshared' parameter annotation added in 202x}}

// expected-warning@-1{{Passing groupshared variable to a parameter annotated with inout. See'groupshared' parameter annotation added in 202x}}

hekota · 2026-01-09T01:57:24Z

tools/clang/test/CodeGenHLSL/groupsharedArgs/TemplateTest.hlsl

+
+// CHECK-LABEL: @"\01?fn1@@YAXAGAU?$Shared@H@@@Z"
+// CHECK: [[D:%.*]] = alloca double, align 8
+// CHECK: [[A:%.*]] = getelementptr inbounds %"struct.Shared<int>", %"struct.Shared<int>" addrspace(3)* %Sh, i32 0, i32 0


Could you please add a CHECK for %Sh before its first use? Where does it come from?

hekota · 2026-01-09T02:08:54Z

tools/clang/test/CodeGenHLSL/groupsharedArgs/Overloads.hlsl

+  fn2(11.xxxx);
+}
+
+void fn(groupshared float4 Arr[2]) {


It would be good to add a CHECK for the groupshared signature functions as well to see how it differs. Also, I think the CHECK-DAG labels in this test should be changed to CHECK (or CHECK-LABEL for the function definitions) because the order matters.

hekota · 2026-01-09T02:25:00Z

tools/clang/test/CodeGenHLSL/groupsharedArgs/ScalarTest.hlsl

+// mangling changes added the first G
+// CHECK-LABEL: @"\01?fn1@@YAXAGAG@Z"


Suggested change

// mangling changes added the first G

// CHECK-LABEL: @"\01?fn1@@YAXAGAG@Z"

// Make sure the groupshared parameter annotation is reflected in the mangled function signature (the first G)

// CHECK-LABEL: @"\01?fn1@@YAXAGAG@Z"

The comment did not make sense to me out of context.

hekota · 2026-01-09T02:26:27Z

tools/clang/lib/AST/HlslTypes.cpp

+  while (pAttributes != nullptr) {
+    switch (pAttributes->getKind()) {
+    case AttributeList::AT_HLSLGroupShared:
+      return true;
+    default:
+      break;
+    }
+    pAttributes = pAttributes->getNext();
+  }


Suggested change

while (pAttributes != nullptr) {

switch (pAttributes->getKind()) {

case AttributeList::AT_HLSLGroupShared:

return true;

default:

break;

}

pAttributes = pAttributes->getNext();

}

while (pAttributes != nullptr) {

if (pAttributes->getKind() == AttributeList::AT_HLSLGroupShared)

return true;

pAttributes = pAttributes->getNext();

}

hekota · 2026-01-09T07:42:54Z

tools/clang/lib/Sema/SemaHLSL.cpp

+        if (!PVD->hasAttrs())
+          continue;
+        for (Attr *A : PVD->getAttrs()) {
+          switch (A->getKind()) {
+          case clang::attr::HLSLGroupShared: {
+            Diag(A->getLocation(), diag::err_hlsl_varmodifiersna)
+                << "groupshared"
+                << "export/noinline"
+                << "parameter";
+            return;
+            break;
+          }
+          }
+        }


Suggested change

if (!PVD->hasAttrs())

continue;

for (Attr *A : PVD->getAttrs()) {

switch (A->getKind()) {

case clang::attr::HLSLGroupShared: {

Diag(A->getLocation(), diag::err_hlsl_varmodifiersna)

<< "groupshared"

<< "export/noinline"

<< "parameter";

return;

break;

}

}

}

if (PVD->hasAttr<HLSLGroupSharedAttr>())

Diag(A->getLocation(), diag::err_hlsl_varmodifiersna)

<< "groupshared"

<< (IsExportAttr ? "export : noinline")

<< "parameter";

return;

}

hekota · 2026-01-09T08:01:30Z

tools/clang/include/clang/AST/Decl.h

+    if (!isModifierIn() && !isModifierOut())
+      return hlsl::ParameterModifier(hlsl::ParameterModifier::Kind::Ref);


It would be better to have an explicit bit for the groupshared modifier on the param decl, the same way it is for out instead of assuming that it is groupshared when it's not in or out.

spall added 4 commits December 3, 2025 16:37

@llvm-beanz proof of concept

e71c32a

get basic tests working

8b20e9e

get compiler to error when types aren't exactly the same + arg not groupshared bug fix fix test disallow groupshared param in export/noinline funcs first check if there are attrs remove accidental change tests

work on errors and warnings

c3437fa

make 202x to avoid adding warning for groupshared arg

fdca3fc

update tests to show new mangling

github-project-automation bot added this to HLSL Roadmap Dec 18, 2025

github-project-automation bot moved this to New in HLSL Roadmap Dec 18, 2025

spall and others added 3 commits January 5, 2026 09:57

self review

c3cadac

attempt to make clang format happy

8511e4e

chore: autopublish 2026-01-06T21:38:05Z

03e93a7

hekota reviewed Jan 9, 2026

View reviewed changes

	// expected-note@-1{{candidate function not viable: 1st argument ('half') is in address space 0, but parameter must be in address space}}
	// expected-note@-1{{candidate function not viable: 1st argument ('half') is in address space 0, but parameter must be in address space 3}}

		RWStructuredBuffer<uint4> Out : register(u0);

		groupshared uint16_t SharedData;

	// expected-warning@-1{{assing groupshared variable to a parameter annotated with inout. See'groupshared' parameter annotation added in 202x}}
	// expected-warning@-1{{Passing groupshared variable to a parameter annotated with inout. See'groupshared' parameter annotation added in 202x}}

		// mangling changes added the first G
		// CHECK-LABEL: @"\01?fn1@@YAXAGAG@Z"

		if (!isModifierIn() && !isModifierOut())
		return hlsl::ParameterModifier(hlsl::ParameterModifier::Kind::Ref);

Implement support for groupshared arg attribute #8013

Are you sure you want to change the base?

Implement support for groupshared arg attribute #8013

Conversation

spall commented Dec 18, 2025

Uh oh!

github-actions bot commented Dec 18, 2025 • edited by spall Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hekota left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Dec 18, 2025 •

edited by spall

Loading